ETT-1189: Disable PHP error reporting in production; ETT-1140 show a styled 404 page #124
Conversation
|
|
||
|
|
||
| RewriteRule ^robots.txt$ static/robots.txt [L] | ||
| RewriteRule ^F static/rewrite.php [L] |
aelkiss
changed the title
|
Playwright indicates that the API paths are returning a 200 when they should return a 404. I'll look at that, and maybe just go ahead and add a playwright test for the default error path in the catalog. |
|
@aelkiss the Bib API 404s in playwright seem to now be squeaking through index.php line 329. If I add |
|
Playwright fix: |
moseshll
left a comment
moseshll
left a comment
There was a problem hiding this comment.
Once the playwright issue is cleared up I'd call this good to go.
|
@moseshll lol were we both working on that playwright issue at the same time? sorry 🙈 |
|
The markup is fine without the styles, but just FYI that the styles for this won't be available until hathitrust/firebird-common#141 is deployed. |
We do not want php error output going to the user.
rewrite.php was removed in 64fd3d4 to no apparent ill effect. The redirect there to mirlyn-classic won't have worked for years, and the other rewrite there (with func=direct and a doc_number parameter) may have served for very obsolete forms of URLs. This may have been vestigial from when this code base was used for the Michigan catalog.
* Don't display PHP error messages on the 404 page * Use as ErrorDocument as well as as the default error handler in catalog
aelkiss
force-pushed
the
ETT-1189-no-error-msgs
branch
from
7aa8371 to
e658822
Compare
Commenting out the
php_valuestuff in.htaccesswas the main thing that gets the errors out of the web page;ErrorDocument 404plus the fixes in the PHP default error handler should handle the remaining 404 errors.To test this:
With an error, we'll get an unstyled default 500 page. This is better than a messy PHP error; I couldn't get
ErrorDocumentto work here, and I'm not sure why, but I also don't know how much it really matters. We can certainly keep an eye out to see what kinds of things trigger 500 errors in production.The fallthrough styled 404 also relies on
ErrorDocumentin.htaccess, so I don't have a good way to test that with playwright. We could test that e.g. "/foo/bar" gives a styled 404 with playwright, I suppose, if we want, since that's being handled by the application. Again, we can try it on dev-N with a variety of garbage URLs, all of which should give the 404 error page.@carylwyatt feel free to apply styling as desired in interface/themes/firebird/Error/Error404.tpl (and perhaps Record/error.tpl and Search/error.tpl?)